Skip to content

Fix NaN/read-only errors in Gaia filler and guide star handling#41

Open
monodera wants to merge 4 commits intomainfrom
u/monodera/fix-nan-gaia-fillers
Open

Fix NaN/read-only errors in Gaia filler and guide star handling#41
monodera wants to merge 4 commits intomainfrom
u/monodera/fix-nan-gaia-fillers

Conversation

@monodera
Copy link
Contributor

@monodera monodera commented Mar 14, 2026

Summary

  • dbutils.py: Fill NaN pmra/pmdec/parallax values for Gaia filler targets in fixcols_gaiadb_to_targetdb(). Gaia sources (especially faint ones) often lack astrometry, and these NaNs propagated into get_fp_positions(), causing the coordinate transform to produce NaN focal plane positions, which then triggered ValueError: data must be finite in TargetSelector.constructKDTree().
  • designutils.py: Pass .copy() of the parallax array to ctrans() in generate_guidestars_from_gaiadb() and generate_guidestars_from_csv(). pfs.utils.Subaru_POPT2_PFS.radec2radecplxpm() modifies the parallax array in-place, which raises ValueError: assignment destination is read-only when Pandas returns a read-only view (default in Pandas 3.x with Copy-on-Write).
  • pyproject.toml: Pin pandas<3.0.0 until external PFS packages are validated against Pandas 3.x Copy-on-Write behavior.
  • Linter fixes (multiple files): Replace bare except with specific exception types (ValueError, FileNotFoundError, KeyError, TypeError, AttributeError); replace == True/False/None comparisons with idiomatic is None, not x, elif x, ~df[col]; use .isin() for the mixed-type is_medium_resolution column; fix E701 inline if statement.

Test plan

  • Run pfs-fiber-allocation end-to-end and confirm no ValueError: data must be finite from KDTree
  • Confirm no ValueError: assignment destination is read-only from radec2radecplxpm
  • Confirm guide stars are correctly generated in the output pfsDesign

🤖 Generated with Claude Code

monodera and others added 3 commits March 13, 2026 14:45
In fixcols_gaiadb_to_targetdb(), Gaia sources with missing astrometry
(common for faint stars) were passed through with NaN proper motion and
parallax values. These NaNs propagated into get_fp_positions(), causing
the coordinate transform to produce NaN focal plane positions, which
then triggered a ValueError in TargetSelector.constructKDTree().

Apply the same NaN-filling used in other fetch functions: pmra/pmdec -> 0,
parallax -> 1e-7 (effectively zero parallax).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…error

pfs.utils.coordinates.Subaru_POPT2_PFS.radec2radecplxpm() modifies the
parallax argument in-place (str_plx[...] = 0.00001). With Pandas 3.x,
DataFrame.to_numpy() can return a read-only view due to Copy-on-Write,
causing a ValueError when the function tries to write to it.

Pass .copy() of the parallax array at both ctrans call sites in
generate_guidestars_from_gaiadb() and generate_guidestars_from_csv().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pandas 3.0 enables Copy-on-Write by default, causing .to_numpy() to
return read-only views. External PFS packages (pfs_utils, ics_cobraOps,
etc.) have not been validated against Pandas 3.x and may have similar
in-place modification issues. Pin to <3.0.0 until the ecosystem catches up.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@monodera
Copy link
Contributor Author

@wanqqq31 @yukimoritani I'm wondering whether filling NaN values as seen in this PR is good for our purpose.

…ptions and idiomatic checks

- Use specific exception types (ValueError, FileNotFoundError, KeyError, TypeError, AttributeError) instead of bare except
- Replace == True/False/None with idiomatic is None, not x, elif x, ~df[col], df[col]
- Use .isin() for mixed-type is_medium_resolution column (str "L/M" + bool)
- Fix E701 inline if statement in reconfigure_fibers_ppp.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant